-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Teach parser to understand fake anonymous enum syntax #106960
Conversation
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
b64e455
to
3e53763
Compare
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt |
Parse `-> Ty | OtherTy`. Parse type ascription in top level patterns.
71a2806
to
c847a01
Compare
You should probably add a macro_rules! check {
($Z:ty) => { compile_error!("triggered"); };
($X:ty | $Y:ty) => { };
}
check! { i32 | u8 } // passes on stable & nightly |
@fmease that code continues to compile successfully (because the new parsing only triggers explicitly on type params and return types and nothing else). Added it as a test, as well as another test for the macro in those positions to confirm that even then the new parsing doesn't trigger in the macro context. |
89dfe4c
to
12d18e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The improvements to type ascription in patterns are great.
compiler/rustc_ast/src/ast.rs
Outdated
@@ -2096,6 +2096,9 @@ pub enum TyKind { | |||
Err, | |||
/// Placeholder for a `va_list`. | |||
CVarArgs, | |||
/// Placeholder for "anonymous enums", which don't exist, but keeping their | |||
/// information around lets us produce better diagnostics. | |||
AnonEnum(Vec<P<Ty>>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I generally encourage to add information to the AST for diagnostics, adding a variant that does not exist in the language seems a bit too far.
IIUC, this is only needed for pretty-printing. I couldn't see where it appears in ui tests. Why isn't the usual TyKind::Err
sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can avoid including this for the purposes of this PR, but I have a very early attempt at real support for anonymous enums 1) to have more info for more accurate suggestions in later stages, particularly to deduplicate them (thinking of declaring the same enum in multiple return types) in subsequent work, and 2) having an in-tree proof of concept of the actual feature to convince myself of whether a very restricted version of this would be possible. I can safely remove this from this PR.
Edit: took it out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe TyKind::Err
could contain an enum with parsed invalid syntax, like
enum TyError {
AnonEnum(Vec<P<Ty>>),
Other,
}
0a8fa43
to
cfff2eb
Compare
This comment was marked as resolved.
This comment was marked as resolved.
cfff2eb
to
020cca8
Compare
@bors r+ |
Teach parser to understand fake anonymous enum syntax Parse `Ty | OtherTy` in function argument and return types. Parse type ascription in top level patterns. Minimally address rust-lang#100741.
Teach parser to understand fake anonymous enum syntax Parse `Ty | OtherTy` in function argument and return types. Parse type ascription in top level patterns. Minimally address rust-lang#100741.
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#106407 (Improve proc macro attribute diagnostics) - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax) - rust-lang#107085 (Custom MIR: Support binary and unary operations) - rust-lang#107086 (Print PID holding bootstrap build lock on Linux) - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`) - rust-lang#107204 (suggest qualifying bare associated constants) - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer ) - rust-lang#107272 (Implement ObjectSafe and WF in the new solver) - rust-lang#107285 (Implement `Generator` and `Future` in the new solver) - rust-lang#107286 (ICE in new solver if we see an inference variable) - rust-lang#107313 (Add Style Team Triagebot config) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…ambiguous, r=estebank Revert "Teach parser to understand fake anonymous enum syntax" and related commits anonymous enum types are currently ambiguous in positions like: * `|` operator: `a as fn() -> B | C` * closure args: `|_: as fn() -> A | B` I first tried to thread around `RecoverAnonEnum` into all these positions, but the resulting complexity in the compiler is IMO not worth it, or at least worth a bit more thinking time. In the mean time, let's revert this syntax for now, so we can go back to the drawing board. Fixes rust-lang#107461 cc: `@estebank` `@cjgillot` rust-lang#106960 --- ### Squashed revert commits: Revert "review comment: Remove AST AnonTy" This reverts commit 020cca8. Revert "Ensure macros are not affected" This reverts commit 12d18e4. Revert "Emit fewer errors on patterns with possible type ascription" This reverts commit c847a01. Revert "Teach parser to understand fake anonymous enum syntax" This reverts commit 2d82420.
…biguous, r=estebank Revert "Teach parser to understand fake anonymous enum syntax" and related commits anonymous enum types are currently ambiguous in positions like: * `|` operator: `a as fn() -> B | C` * closure args: `|_: as fn() -> A | B` I first tried to thread around `RecoverAnonEnum` into all these positions, but the resulting complexity in the compiler is IMO not worth it, or at least worth a bit more thinking time. In the mean time, let's revert this syntax for now, so we can go back to the drawing board. Fixes rust-lang#107461 cc: `@estebank` `@cjgillot` rust-lang#106960 --- ### Squashed revert commits: Revert "review comment: Remove AST AnonTy" This reverts commit 020cca8. Revert "Ensure macros are not affected" This reverts commit 12d18e4. Revert "Emit fewer errors on patterns with possible type ascription" This reverts commit c847a01. Revert "Teach parser to understand fake anonymous enum syntax" This reverts commit 2d82420.
…ler-errors Parse and recover from type ascription in patterns Reintroduce part of rust-lang#106960, which was reverted in rust-lang#107478. r? `@compiler-errors`
…iaskrgr Rollup of 11 pull requests Successful merges: - rust-lang#106407 (Improve proc macro attribute diagnostics) - rust-lang#106960 (Teach parser to understand fake anonymous enum syntax) - rust-lang#107085 (Custom MIR: Support binary and unary operations) - rust-lang#107086 (Print PID holding bootstrap build lock on Linux) - rust-lang#107175 (Fix escaping inference var ICE in `point_at_expr_source_of_inferred_type`) - rust-lang#107204 (suggest qualifying bare associated constants) - rust-lang#107248 (abi: add AddressSpace field to Primitive::Pointer ) - rust-lang#107272 (Implement ObjectSafe and WF in the new solver) - rust-lang#107285 (Implement `Generator` and `Future` in the new solver) - rust-lang#107286 (ICE in new solver if we see an inference variable) - rust-lang#107313 (Add Style Team Triagebot config) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Teach parser to understand fake anonymous enum syntax Parse `Ty | OtherTy` in function argument and return types. Parse type ascription in top level patterns. Minimally address rust-lang#100741.
Parse
Ty | OtherTy
in function argument and return types.Parse type ascription in top level patterns.
Minimally address #100741.